Add typing for perspective#157
Conversation
| } | ||
|
|
||
| export type ValuesByType = { | ||
| [ key in TypeNames ]: Array<string> |
There was a problem hiding this comment.
For primitives it's better to use string[] for arrays.
There was a problem hiding this comment.
https://palantir.github.io/tslint/rules/array-type/
we use "generic" on our project, hence Array<T> over T[]
but happy to change if you want me to
There was a problem hiding this comment.
Sure, since there's no tslint setup of similar and they're equivalent let's just go with this for now. We can always change later - I just normally go with "array-simple" but consistency makes sense.
| NONE = 'none', | ||
| } | ||
|
|
||
| enum NUMBER_AGGREGATES { |
There was a problem hiding this comment.
These might potentially be better as Enums rather than strings?
There was a problem hiding this comment.
they are enums, or you mean to create a global enum with all of them and NUMBER, STRING, BOOLEAN to be just a subset of them?
There was a problem hiding this comment.
I was thinking that maybe we can take the aggregate keywords and Enum those. But I couldn't find a nice way to make them all align up. Since these are private I'll take another look later and we can hoist them into private enums later.
|
given that the jupyterlab sub package is already in typescript, typing should also be added there (instead of the current <any> casts). |
|
Looks great, thanks! |
|
shouldn't the jupyterlab package use this |
4 participants
No description provided.